Skip to content
This repository has been archived by the owner on Feb 5, 2020. It is now read-only.

Ensure sized on associated methods #6

Merged

Conversation

macisamuele
Copy link
Contributor

@macisamuele macisamuele commented Jan 12, 2019

Associated methods needs to be available only for Sized implementations.
As far as I can tell this is a common pattern for traits that adds associated methods to structs (ie. Default, From, etc.)

The main rationale of this change is to allow NamedType derived structs to be stored into structures like Vec<Box<a_trait_name>>

The added test case would fail to compile if the changes into named_type/src/lib.rs would be reverted, in such way we can prevent eventual regressions.

I've also added named_type_derive dependency in named_type such that users of this crate will need to only add named_type dependency in cargo.
Without doing that users were forced to use named_type and named_type_derive and to ensure that both were defined with the same version.

@macisamuele macisamuele force-pushed the maci-ensure-sized-on-associated-methods branch from 32ff8c2 to 345a358 Compare January 12, 2019 15:09
@cjhowedev
Copy link
Owner

Everything looks good to me. Just one thing:

I've also added named_type_derive dependency in named_type such that users of this crate will need to only add named_type dependency in cargo.
Without doing that users were forced to use named_type and named_type_derive and to ensure that both were defined with the same version.

I don't have any problem with this, but if both named_type and named_type_derive have the same version, I don't think I can push them both to crates.io. One of them will have a dependency for a version which doesn't yet exist, since I push them to crates.io serially. I don't know if it's possible to push both versions in parallel, but I'll look into it. It's possible the cargo workspace helps with this, and last time I tested it that wasn't in place. I'm busy tonight, but I'll see if I can get it to work tomorrow!

@macisamuele
Copy link
Contributor Author

@cjhowe7 Happy to revert the last commit so this could be unlocked and eventually revisit it later.

@macisamuele
Copy link
Contributor Author

btw checking https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-path-dependencies looks like it would eventually be possible defining a path dependency but in order to make it upload-able it requires also a valid version that will be then fetched from crates.io

So it should be something like

# named_type/Cargo.toml
version = "xxx"
[dependencies]
named_type_derive = { path = "../named_type_derive", version = "xxx" }

it will require some synchronisation as named_type_derive needs to be upload before named_type

@macisamuele
Copy link
Contributor Author

@cjhowe7 any update on this?

@cjhowedev
Copy link
Owner

Oh my bad, I forgot about this. Just tested this locally, I think everything looks good. Merging now.

@cjhowedev cjhowedev merged commit 8f0162f into cjhowedev:master Jan 30, 2019
@macisamuele
Copy link
Contributor Author

No problem :) thanks a lot for merging it

@macisamuele macisamuele deleted the maci-ensure-sized-on-associated-methods branch January 30, 2019 17:31
@cjhowedev
Copy link
Owner

Published in https://crates.io/crates/named_type/0.2.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants